fix(api): fall back to WEB_URL when Redis origin key is missing (#9354)#9361
Conversation
…plane#9354) send_email_notification silently dropped email notifications (while still reporting task success) whenever the Redis-cached origin key for an issue was missing due to TTL expiry, a Redis restart, or the activity being queued without an origin. This change replaces the silent early return with a fallback to settings.WEB_URL for link construction, plus a warning log so the condition is observable. Fixes makeplane#9354
reachsanjivbhagat-gif
requested review from
dheeru0198 and
pablohashescobar
as code owners
|
|
📝 WalkthroughWalkthroughThe ChangesEmail Notification Fallback
Estimated code review effort: 1 (Trivial) | ~5 minutes Related issues: Fixes Suggested labels: bug, backend Suggested reviewers: none Poem A rabbit hopped through Redis' door, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/api/plane/bgtasks/email_notification_task.py (1)
167-172: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winFallback correctly avoids silent drop, but
settings.WEB_URLisn't validated.This correctly addresses the PR objective of not silently dropping notifications. However,
settings.WEB_URLis derived fromos.environ.get("WEB_URL")with no default, so if it's unset/empty in a given deployment, the fallback substitutes one falsy value for another and the generated links (avatar/issue/project URLs) will be malformed (e.g.,"None/<workspace>/projects/...") while the email still sends successfully.Consider tightening the warning to also flag when the fallback itself is unusable:
💡 Suggested tweak
if not base_api: - logging.getLogger("plane.worker").warning( - f"Redis origin key missing for issue {issue_id}; falling back to WEB_URL for email notification links" - ) base_api = settings.WEB_URL + if not base_api: + logging.getLogger("plane.worker").error( + f"Redis origin key missing for issue {issue_id} and settings.WEB_URL is also unset; " + "email notification links may be malformed" + ) + else: + logging.getLogger("plane.worker").warning( + f"Redis origin key missing for issue {issue_id}; falling back to WEB_URL for email notification links" + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/plane/bgtasks/email_notification_task.py` around lines 167 - 172, The fallback in email_notification_task’s base URL handling is still unsafe because settings.WEB_URL can be unset or empty, leading to malformed links while notifications still send. Update the logic in the email notification flow around the base_api fallback so it validates settings.WEB_URL before assigning it, and if both the cached origin and WEB_URL are unusable, log a clear warning through plane.worker and avoid building broken avatar/issue/project URLs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/api/plane/bgtasks/email_notification_task.py`:
- Around line 167-172: The fallback in email_notification_task’s base URL
handling is still unsafe because settings.WEB_URL can be unset or empty, leading
to malformed links while notifications still send. Update the logic in the email
notification flow around the base_api fallback so it validates settings.WEB_URL
before assigning it, and if both the cached origin and WEB_URL are unusable, log
a clear warning through plane.worker and avoid building broken
avatar/issue/project URLs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7caadd8a-122d-4162-9a76-b62473fe9532
📒 Files selected for processing (1)
apps/api/plane/bgtasks/email_notification_task.py
2 participants
Description
send_email_notificationinapps/api/plane/bgtasks/email_notification_task.pysilently dropped email notifications, while still reporting the Celery task as successful, whenever the Redis-cached "origin" key for an issue was missing. This happens whenever Redis restarts/evicts the key, more than 10 minutes (the key's TTL) pass before the email task runs, or the triggering activity was queued without an origin at all.This PR replaces the silent early
returnwith a fallback tosettings.WEB_URLfor constructing links in the email, and adds alogging.warningcall so the condition is observable in logs instead of failing silently.Type of Change
Bug fix (non-breaking change which fixes an issue).
Screenshots and Media (if applicable)
Not applicable, this is a backend logic change with no UI impact.
Test Scenarios
Manually traced the code path: with the Redis key absent,
base_apinow falls back tosettings.WEB_URLand a warning is logged instead of the function returning early. Confirmed the rest ofsend_email_notification(payload creation, email rendering, and sending) proceeds unchanged after the fallback.References
Fixes #9354
Summary by CodeRabbit